Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-add py3.9 support to sage_autodoc.py #38619

Merged
merged 5 commits into from
Sep 15, 2024
Merged

Conversation

kiwifb
Copy link
Member

@kiwifb kiwifb commented Sep 4, 2024

This PR corrects the fact the sage_autodoc.py changes needed for sphinx 8 dropped python 3.9 support. This is a consequence of the fact that sphinx 8 has dropped python 3.9.
Since we still want to support python 3.9, some support has to be re-introduced.
See #38549 (comment) and after.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

@kiwifb kiwifb mentioned this pull request Sep 4, 2024
5 tasks
@mkoeppe
Copy link

mkoeppe commented Sep 5, 2024

In the CI, I see:

  [sagemath_doc_html-none] [spkg-install] [algebras ]   File "/sage/src/sage_docbuild/ext/sage_autodoc.py", line 1909, in get_doc
  [sagemath_doc_html-none] [spkg-install] [algebras ]     if sys.version_info[:2] < (3, 10):
  [sagemath_doc_html-none] [spkg-install] [algebras ] NameError: name 'sys' is not defined

@kiwifb
Copy link
Member Author

kiwifb commented Sep 5, 2024

I should have double checked that an import of sys was present.

Copy link

github-actions bot commented Sep 5, 2024

Documentation preview for this PR (built with commit 57f5d16; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@kiwifb
Copy link
Member Author

kiwifb commented Sep 5, 2024

Looks a lot healthier now.

@mkoeppe mkoeppe added this to the sage-10.5 milestone Sep 5, 2024
@mkoeppe
Copy link

mkoeppe commented Sep 5, 2024

This error still shows up with Python 3.9:

[spkg-install] [spkg     ] Exception occurred:
[spkg-install] [spkg     ]   File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-pristine/src/sage_docbuild/ext/sage_autodoc.py", line 1654, in import_object
[spkg-install] [spkg     ]     if isinstance(self.object, NewType | TypeVar):
[spkg-install] [spkg     ] TypeError: unsupported operand type(s) for |: 'function' and 'type'
[spkg-install] [spkg     ] The full traceback has been saved in /var/folders/38/wnh4gf1552g_crsjnv2vmmww0000gp/T/sphinx-err-0l5np5ke.log, if you want to report th

@kiwifb
Copy link
Member Author

kiwifb commented Sep 5, 2024

Hopefully, I will not have to make those into blocks conditional to the version of python. And they will be enough.

@mkoeppe
Copy link

mkoeppe commented Sep 5, 2024

[sagemath_doc_html-none] [spkg-install] [combinat ] Exception occurred:
[sagemath_doc_html-none] [spkg-install] [combinat ]   File "/Users/mkoeppe/s/sage/sage-rebasing/worktree-pristine/src/sage_docbuild/ext/sage_autodoc.py", line 674, in add_content
[sagemath_doc_html-none] [spkg-install] [combinat ]     for line, src in zip(more_content.data, more_content.items, strict=True):
[sagemath_doc_html-none] [spkg-install] [combinat ] TypeError: zip() takes no keyword arguments

@mkoeppe
Copy link

mkoeppe commented Sep 6, 2024

This does the trick, thanks very much!

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 8, 2024
    
This PR corrects the fact the sage_autodoc.py changes needed for sphinx
8 dropped python 3.9 support. This is a consequence of the fact that
sphinx 8 has dropped python 3.9.
Since we still want to support python 3.9, some support has to be re-
introduced.
See sagemath#38549 (comment)
and after.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#38619
Reported by: François Bissey
Reviewer(s):
@kiwifb
Copy link
Member Author

kiwifb commented Sep 8, 2024

And it breaks sphinx-8. I will need to add some try expect blocks. Since it is being merged, that will be another follow up.

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 8, 2024
    
This PR corrects the fact the sage_autodoc.py changes needed for sphinx
8 dropped python 3.9 support. This is a consequence of the fact that
sphinx 8 has dropped python 3.9.
Since we still want to support python 3.9, some support has to be re-
introduced.
See sagemath#38549 (comment)
and after.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#38619
Reported by: François Bissey
Reviewer(s):
@kiwifb
Copy link
Member Author

kiwifb commented Sep 8, 2024

@vbraun I see you have this in your merge tree, but it turns out it needs more work. Is it OK with you if I push some more commits or you'd prefer a follow up?

@vbraun
Copy link
Member

vbraun commented Sep 9, 2024

you can keep working on it

@kiwifb
Copy link
Member Author

kiwifb commented Sep 9, 2024

@mkoeppe if you would like to check that it is still working for you on py3.9. Unless you find something not working, it should be the last tweak.

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 10, 2024

It works well with py3.9. LGTM.

@kiwifb
Copy link
Member Author

kiwifb commented Sep 10, 2024

Thanks!

vbraun pushed a commit to vbraun/sage that referenced this pull request Sep 12, 2024
    
This PR corrects the fact the sage_autodoc.py changes needed for sphinx
8 dropped python 3.9 support. This is a consequence of the fact that
sphinx 8 has dropped python 3.9.
Since we still want to support python 3.9, some support has to be re-
introduced.
See sagemath#38549 (comment)
and after.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.
    
URL: sagemath#38619
Reported by: François Bissey
Reviewer(s): Kwankyu Lee
@vbraun vbraun merged commit bcedc6a into sagemath:develop Sep 15, 2024
15 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants